cli: Name the real Bitcoin Core requirement when it is missing - #99
BenWestgate wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b738eb71e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
a7efaae to
054e8d9
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
054e8d9 to
115f2c2
Compare
Without bitcoin-cli every ms32 command said "Install a reviewed bitcoin-cli before creating a backup", even commands that create nothing, and the codex32 hint didn't say what it leaves out. Say that Bitcoin Core 32 or newer must run with RPC enabled and that an unsynced regtest or signet node is enough for practice. Then say what the command uses Core for: the master fingerprint and correction ranking for secret, share and correct (with the codex32 fallback and what it omits), or giving Core the master key for create and wallet. ms32 correct now connects before its search instead of after it, so a missing Core no longer costs up to ten seconds of discarded work. Closes #84 Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
Connecting before the search made two problems easier to hit. With damaged input piped to `ms32 correct` and two Core networks running, the network prompt read the exhausted pipe forever. With stderr redirected, "Using Bitcoin Core on ..." and a blank line came before `interactive confirmation required`, which the security model says must be the only message. Ask for a network only when stdin is a terminal; a pipe now gets "More than one local Bitcoin Core network is running." Print Core's messages only when stderr is a terminal. The gate test's fake now reports like the real one, so it catches the extra output. Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
d422478 to
c583b24
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Ben authorized raising the budget so #91 fits. The stack tip with the open fix PRs was at 5,197 of 5,200, and #91 adds 26 lines. Update the enforcing test and both places that document the number. Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
CACK, will review the code after the bots say it is ready for me. |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Requested by Ben · project thread
Before: without
bitcoin-cli,ms32 secret,share,correct,walletandcreateall said "Install a reviewed bitcoin-cli before creating a backup." Thecodex32 <command>hint didn't say what it leaves out, andms32 correctsearched for up to ten seconds before failing.After:
createandwalletsay "This command gives Bitcoin Core the master key." with no fallback. The no-RPC-server message also names Core 32.ms32 correctconnects before searching.How:
_bitcoin_core.pygets the new messages and_connected_core()incli.pyadds the second line. In_correct, the Core connection moves from after the search to just after the "already valid" check. Tests cover all three fallbacks (with the search forbidden forcorrect) and the no-fallback wallet path.Budget: 5192 of < 5200 (#57 is 5189). 927 tests pass; ruff, mypy and the touched files under
-Oare clean.Based on #57 because it changes the same connection path. Closes #84
🤖 Generated with Claude Code
https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
Generated by Claude Code